Skip to content

Stabilize s390x vector target feature and is_s390x_feature_detected! macro #145656

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Stabilize s390x vector target feature and is_s390x_feature_detected! macro #145656

wants to merge 5 commits into from
+46 −78

Conversation

folkertdev
Copy link
Contributor

@folkertdev folkertdev commented Aug 20, 2025 •
edited
Loading

closes #145649
closes #135413
cc: #130869
reference PR: rust-lang/reference#1972

Stabilization report

Summary

This PR stabilizes the following s390x target features:

  • vector
  • vector-enhancements-1
  • vector-enhancements-2
  • vector-enhancements-3
  • vector-packed-decimal
  • vector-packed-decimal-enhancement
  • vector-packed-decimal-enhancement-2
  • vector-packed-decimal-enhancement-3
  • nnp-assist
  • miscellaneous-extensions-2
  • miscellaneous-extensions-3
  • miscellaneous-extensions-4

Additionally, it stabilizes the core::arch::is_s390x_feature_detected! macro itself and stably accepts the target features listed above.

Tests & ABI details

Only the vector target feature changes the ABI, much like e.g. avx2 it will, depending on the ABI, pass vector types in vector registers. This behavior is tested extensively:

The remaining features don't influence the ABI, they only influence instruction selection. In stdarch we test that the expected instructions are in fact generated when the target feature is enabled.

Implementation history

For is_s390x_feature_detected!:

For vector and friends

Unresolved questions

There is a fixme in tests/ui/abi/simd-abi-checks-s390x.rs:

// FIXME: +soft-float itself doesn't set -vector
//@[z13_soft_float] compile-flags: --target s390x-unknown-linux-gnu -C target-cpu=z13 -C target-feature=-vector,+soft-float
//@[z13_soft_float] needs-llvm-components: systemz

I'm not sure whether that blocks stabilization?

The implementation first extracts the listed target features into their own s390x_target_feature_vector rust feature, and then stabilizes that. best reviewed commit-by-commit

r? @Amanieu
cc @uweigand @taiki-e

@folkertdev folkertdev added O-SystemZ Target: SystemZ processors (s390x) I-lang-nominated Nominated for discussion during a lang team meeting. I-libs-nominated Nominated for discussion during a libs team meeting. labels Aug 20, 2025
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Aug 20, 2025
@rustbot
Copy link
Collaborator

rustbot commented Aug 20, 2025

stdarch is developed in its own repository. If possible, consider making this change to rust-lang/stdarch instead.

cc @Amanieu, @folkertdev, @sayantn

@Amanieu Amanieu added T-lang Relevant to the language team T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. and removed T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. I-libs-nominated Nominated for discussion during a libs team meeting. labels Aug 20, 2025
@Amanieu
Copy link
Member

Amanieu commented Aug 20, 2025

@rfcbot merge

@rfcbot
Copy link
Collaborator

rfcbot commented Aug 20, 2025 •
edited
Loading

Team member @Amanieu has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns.
See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Aug 20, 2025
@Amanieu
Copy link
Member

Amanieu commented Aug 20, 2025

There is a fixme in tests/ui/abi/simd-abi-checks-s390x.rs:

// FIXME: +soft-float itself doesn't set -vector
//@[z13_soft_float] compile-flags: --target s390x-unknown-linux-gnu -C target-cpu=z13 -C target-feature=-vector,+soft-float
//@[z13_soft_float] needs-llvm-components: systemz

I'm not sure whether that blocks stabilization?

I think this is fine since we don't have a soft-float s390x target and don't expose the soft-float feature. However we would want to re-visit this if we ever get such a target.

@folkertdev folkertdev mentioned this pull request Aug 20, 2025
Open
@traviscross traviscross added the P-lang-drag-1 Lang team prioritization drag level 1. https://rust-lang.zulipchat.com/#narrow/channel/410516-t-lang label Aug 20, 2025
@RalfJung
Copy link
Member

+soft-float doesn't usually disable anything, it's about the ABI entirely orthogonal to which registers are available. So I think that FIXME should just be removed.

@uweigand
Copy link
Contributor

uweigand commented Aug 20, 2025 •
edited
Loading

On s390x, in LLVM (and GCC), the +soft-float feature does in fact disable any use of floating-point (or vector) registers, and therefore implicitly any use of instructions requiring these features, which implies e.g. fully disabling the +vector feature. This is in addition to the ABI changes (which are of course necessary as the default ABI makes use of floating-point and vector registers).

In LLVM in particular, the back-end does actually implement that +soft-float implies -vector. I guess the above FIXME is about making sure this matches what Rust itself assumes? Either way, I think resolution of this can wait until we're trying to expose the feature.

In practice, the only user of +soft-float on our platform would be the Linux kernel. Here, we don't really care about the ABI aspect (the kernel does not use any floating-point or vector data types), but we do care about the guarantee that generated code will not touch floating-point or vector registers.

@traviscross
Copy link
Contributor

We've revised and approved the Reference PR, so this will be good to go on that front when FCP completes.

(cc @rust-lang/lang-docs)

On the lang side, this looks good to me.

@rfcbot reviewed

@traviscross traviscross added the I-lang-radar Items that are on lang's radar and will need eventual work or consideration. label Aug 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@Amanieu Amanieu

Labels
disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. I-lang-nominated Nominated for discussion during a lang team meeting. I-lang-radar Items that are on lang's radar and will need eventual work or consideration. O-SystemZ Target: SystemZ processors (s390x) P-lang-drag-1 Lang team prioritization drag level 1. https://rust-lang.zulipchat.com/#narrow/channel/410516-t-lang proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-lang Relevant to the language team T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Tracking Issue for s390x_target_feature_vector Tracking Issue for stdarch_s390x_feature_detection
7 participants
@folkertdev @rustbot @Amanieu @rfcbot @RalfJung @uweigand @traviscross